-
Notifications
You must be signed in to change notification settings - Fork 175
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add strict type coding standard #6039
Add strict type coding standard #6039
Conversation
Blocked by #5356. Otherwise, when I try to update the |
#5356 is an issue (which I think is already resolved and needs to be closed?) |
|
||
<rule ref="SlevomatCodingStandard.TypeHints.LongTypeHints"/> | ||
|
||
<rule ref="SlevomatCodingStandard.TypeHints.ReturnTypeHintSpacing"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this have to do with enforcing strict types?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It makes the spacing of type hints consistent. More of an aesthetic thing. Do you want me to remove it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's already passing and didn't require any changes I guess it's fine, but the PR title says it's for adding strict_type.
|
||
<rule ref="SlevomatCodingStandard.TypeHints.NullableTypeForNullDefaultValue"/> | ||
|
||
<rule ref="SlevomatCodingStandard.TypeHints.ParameterTypeHintSpacing"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or this?
Previous attempts to force type hints or strict typing en masse have failed because it's just too darn complicated to do it all at once. This introduces a new Strict Type coding standard that we can selectively apply to files by adding to an array in test/run-php-linter.sh. Hopefully this will provide an approach to using stricter typing that is easier to author and review. Using vendor/bin/phpcbf --standard=test/StrictTypesCS.xml $file will now allow PHPCBF to automatically provide return types param types strict_types =1 based on PHPDocs. It will also throw errors if Traversable types do not have specific type annotations. The Database.class.inc file has been modified as an example of the changes Resolves aces#5342
Brief summary of changes
Previous attempts to force type hints or strict typing en masse have failed because it's just too darn complicated to do it all at once.
This introduces a new Strict Type coding standard that we can selectively apply to files by adding to an array in
test/run-php-linter.sh
. Hopefully this will provide an approach to using stricter typing that is easier to author and review.Using
vendor/bin/phpcbf --standard=test/StrictTypesCS.xml $file
will now allow PHPCBF to automatically providebased on PHPDocs.
It will also throw errors if Traversable types do not have specific type annotations.
The Database.class.inc file has been modified as an example of the changes
Related
Resolves #5342